Upstream merge#102
Merged
Merged
Conversation
Add repo rules validation to the Rename Branch dialog, matching the existing Create Branch dialog behavior. When a user types a new branch name, it is validated against the repository's branch name pattern rulesets via the API. - Added accounts and cachedRepoRulesets props to RenameBranch component - Added debounced checkBranchRules method (500ms) that calls the GitHub API to fetch and validate branch rules - Shows InputError when the user cannot bypass a failing rule (blocks rename) - Shows InputWarning when the user can bypass (allows rename with caution) - Connected aria-describedby for screen reader accessibility - Passed new props from app.tsx popup rendering
Prevents checkBranchRules from firing after the dialog is dismissed, matching the Create Branch dialog's cleanup pattern.
Set a fixed width of 400px on the rename-branch dialog to match the create-branch dialog. Without this, the dialog expands from min-width to accommodate error/warning text when repo rules validation fires. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Call checkBranchRules in componentDidMount so that when the rename branch dialog opens with a pre-filled name that violates repo rules, the error/warning is shown immediately rather than only after typing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add support for running Playwright E2E tests against an 'unpackaged' app and make packaged the default in CI. Updates: CI workflow runs packaged E2E step; .gitignore ignores playwright-videos; e2e fixtures gain DESKTOP_E2E_APP_MODE handling and unified launch option logic; package.json adds distinct packaged/unpackaged build/run scripts; build script honors DESKTOP_SKIP_PACKAGE to skip packaging when building unpackaged tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a CI step to always upload end-to-end test artifacts (playwright videos) after packaged E2E smoke tests. Uses actions/upload-artifact@v4 and names artifacts by matrix friendlyName and arch; warns if no files found.
Add two composite GitHub Actions to centralize repeated CI steps: setup-ci-environment (Python/Node/ffmpeg/install deps) and setup-windows-signing (install Azure Code Signing client + OIDC login). Update .github/workflows/ci.yml to use these reusable actions and pass the appropriate inputs, reducing duplication and making Windows signing and ffmpeg installation configurable.
Add comprehensive documentation for the new Playwright-based E2E smoke test harness at docs/technical/e2e-smoke-tests.md, describing coverage, launch modes, CI integration, updater mocking, artifacts, and usage. Remove the deprecated script/playwright-electron.ts helper which is no longer used by the test harness. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move checkBranchNameRules() and renderBranchNameRuleError() into a shared utility at ui/lib/branch-name-rule-validation.tsx. Both the create branch and rename branch dialogs now use these shared functions, eliminating ~80 lines of duplicated logic. The shared module also exports IBranchRuleError as a reusable type for the error state shape both dialogs use. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a platform check to dismissMoveToApplicationsDialog so it returns immediately when not running on Darwin (macOS). This prevents attempting to locate macOS-specific dialog buttons on other platforms during e2e tests.
Insert a 'Prepare testing environment' step into .github/workflows/ci.yml to run `yarn test:setup` before the Windows signing action. The step passes npm_config_arch from the matrix to ensure the test environment is prepared for the correct architecture.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Add runner.os == 'Windows' condition to the setup-windows-signing action in CI. This change wraps the action in an if check in two job steps so the Windows code-signing setup only runs on Windows runners, preventing it from executing on non-Windows environments.
Add `id-token: write` to the job permissions in .github/workflows/ci.yml so the workflow can request OIDC tokens for cloud provider authentication and secure deployments. This change enables actions that exchange GitHub OIDC tokens (e.g., to assume roles) while keeping the permission scoped to the CI jobs.
Add utilities to determine release channel and centralize 'Check for Updates' clicking in app-launch E2E tests. Tests now skip startup update checks for non-auto-update channels and reuse clickCheckForUpdatesIfAvailable to avoid duplicated locator logic and brittle checks.
Replace a single textContent read and manual wait with Playwright's expect.poll to repeatedly check the About dialog update status. This makes the e2e check more robust by polling up to 15s (1s intervals) for the message "you have the latest version" in app/test/e2e/app-launch.e2e.ts.
…paths Add E2E tests for critical paths
Enforce repository branch name rules in Rename Branch dialog
There was a problem hiding this comment.
Pull request overview
This PR introduces a Playwright-based E2E smoke-test harness for GitHub Desktop (including CI wiring to run it on macOS/Windows), plus refactors branch name repo-rules validation into a shared helper and applies it to the Rename Branch dialog.
Changes:
- Add Playwright Electron E2E smoke tests, fixtures, mock update server, and documentation.
- Update CI workflow to run the new E2E smoke job and factor common setup/signing steps into composite actions.
- Refactor branch-name repo-rules validation into a shared module and apply it to Rename Branch; add an E2E-specific updates URL override and an option to skip packaging for “unpackaged” E2E runs.
Reviewed changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Adds Playwright-related dependencies and their transitive locks. |
| package.json | Adds E2E scripts and Playwright devDependencies. |
| script/post-install.ts | Adds Playwright ffmpeg installation during postinstall. |
| script/build.ts | Adds DESKTOP_SKIP_PACKAGE option to stage without packaging. |
| app/app-info.ts | Allows overriding updates URL via DESKTOP_E2E_UPDATES_URL for E2E builds. |
| app/test/e2e/app-launch.e2e.ts | Adds the E2E smoke spec (launch flow, repo actions, updater behaviors). |
| app/test/e2e/e2e-fixtures.ts | Adds worker-scoped Playwright Electron fixtures, tracing/video, mock-server helpers. |
| app/test/e2e/mock-update-server.ts | Adds in-process mock update server/control plane for updater tests. |
| app/test/e2e/playwright.config.ts | Adds Playwright config for the E2E suite. |
| app/test/e2e/test-helpers.ts | Adds helpers to create/inspect a throwaway smoke Git repo. |
| docs/technical/e2e-smoke-tests.md | Documents how the E2E harness works and how to run it locally/CI. |
| .github/workflows/ci.yml | Adds an e2e-smoke job and refactors setup/signing steps to composite actions. |
| .github/actions/setup-ci-environment/action.yml | New composite action for Python/Node setup + dependency install (+ optional ffmpeg). |
| .github/actions/setup-windows-signing/action.yml | New composite action encapsulating Windows signing prerequisites + OIDC login. |
| .gitignore | Ignores playwright-videos/ output directory. |
| app/src/ui/lib/branch-name-rule-validation.tsx | New shared helper to check repo rules for branch naming and render errors/warnings. |
| app/src/ui/create-branch/create-branch-dialog.tsx | Replaces inline repo-rules validation with the shared helper. |
| app/src/ui/rename-branch/rename-branch-dialog.tsx | Adds repo-rules validation and error rendering for Rename Branch. |
| app/src/ui/app.tsx | Wires accounts and cachedRepoRulesets through to Rename Branch popup. |
| app/styles/ui/_dialog.scss | Adjusts Rename Branch dialog width. |
| script/default-to-tls12-on-appveyor.reg | Removes unused legacy AppVeyor registry tweak file. |
pol-rivero
force-pushed
the
upstream-merge
branch
2 times, most recently
from
ebae1af to
92f25e6
Compare
pol-rivero
force-pushed
the
upstream-merge
branch
from
92f25e6 to
c2f47e1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Triggering the CI pipeline to see if the new E2E tests work out of the box.